Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use improved high numerical precision linspace from Base #52

Merged
merged 1 commit into from
Jun 10, 2018

Conversation

tlnagy
Copy link
Contributor

@tlnagy tlnagy commented Jun 3, 2018

KernelDensity.jl rolled its own range generator which suffers from similar flaws as Base's linspace prior to being fixed in JuliaLang/julia#18777. This closes #39. @andreasnoack @simonbyrne. This needed for GiovineItalia/Gadfly.jl#1157

Master

julia> length(KernelDensity.kde_range((0.12698109160784082, 0.9785547869337731), 256))
255

This PR

julia> length(KernelDensity.kde_range((0.12698109160784082, 0.9785547869337731), 256))
256

@tlnagy
Copy link
Contributor Author

tlnagy commented Jun 3, 2018

Tests are passing on 0.6 and the failures on the nightlies appears unrelated.

@@ -66,8 +66,7 @@ function kde_range(boundary::Tuple{Real,Real}, npoints::Int)
lo, hi = boundary
lo < hi || error("boundary (a,b) must have a < b")

step = (hi - lo) / (npoints-1)
lo:step:hi
linspace(lo, hi, npoints)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linspace is deprecated in 0.7, so this should be Compat.range(lo, stop=hi, length=npoints)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

KernelDensity.jl rolled its own range generator which suffers from
similar flaws as Base's linspace prior to being fixed in
JuliaLang/julia#18777. This closes JuliaStats#39.
@ararslan ararslan merged commit a0a0859 into JuliaStats:master Jun 10, 2018
@tlnagy tlnagy deleted the tn/kde-range-fix branch June 10, 2018 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug in kde_range
2 participants